Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mountinfo: linux: add a /proc/self/mountinfo fallback #135

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

cyphar
Copy link
Contributor

@cyphar cyphar commented Nov 6, 2023

In very specific cases (that we hit in runc), it is possible to be running in a child pid namespace (where the Go process's pid is 1) but still have a host /proc. While this is not a problem with /proc/thread-self (since the process is in the host pid namespace), the /proc/self/task/ fallback doesn't work because gettid(2) returns the wrong thread-id for the host pid namespace.

There isn't much we can do in this case other than use /proc/self, because the simple workarounds (grep NSpid /proc/self/task/*/status) aren't available in pre-3.17 kernels and any more complicated schemes (such as setting the signal mask to something unique and scanning for it) are unworkable for obvious reasons.

Fixes: 12c61a3 ("mountinfo: linux: use /proc/thread-self/mountinfo")
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

@cyphar
Copy link
Contributor Author

cyphar commented Nov 6, 2023

This should fix the CI issues in opencontainers/runc#4109. Sorry for the noise 😅.

Copy link
Member

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

(because I didn't know offhand if os.IsNotExist would work with nil errs: https://go.dev/play/p/Hku5gFHXWtX)

In very specific cases (that we hit in runc), it is possible to be
running in a child pid namespace (where the Go process's pid is 1) but
still have a host /proc. While this is not a problem with
/proc/thread-self (since the process *is* in the host pid namespace),
the /proc/self/task/<tid> fallback doesn't work because gettid(2)
returns the wrong thread-id for the host pid namespace.

There isn't much we can do in this case other than use /proc/self,
because the simple workarounds (grep NSpid /proc/self/task/*/status)
aren't available in pre-3.17 kernels and any more complicated schemes
(such as setting the signal mask to something unique and scanning for
it) are unworkable for obvious reasons.

Fixes: 12c61a3 ("mountinfo: linux: use /proc/thread-self/mountinfo")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
@cyphar cyphar force-pushed the mountinfo-linux-proc-self-fallback branch from f0e0b5e to 6c165c1 Compare November 6, 2023 12:41
@cyphar
Copy link
Contributor Author

cyphar commented Nov 6, 2023

Yup, it seems this does fix the issue. Before:

=== RUN   TestUsernsCheckpoint
    checkpoint_test.go:95: unexpected error: unable to start container process: error during container init: error preparing rootfs: open /proc/self/task/1/mountinfo: no such file or directory
--- FAIL: TestUsernsCheckpoint (0.28s)
=== RUN   TestCheckpoint
    checkpoint_test.go:95: unexpected error: unable to start container process: error during container init: error preparing rootfs: open /proc/self/task/1/mountinfo: no such file or directory
--- FAIL: TestCheckpoint (0.21s)
=== RUN   TestExecPS
    exec_test.go:43: |: unable to start container process: error during container init: error preparing rootfs: open /proc/self/task/1/mountinfo: no such file or directory
--- FAIL: TestExecPS (0.15s)
=== RUN   TestUsernsExecPS
    exec_test.go:43: |: unable to start container process: error during container init: error preparing rootfs: open /proc/self/task/1/mountinfo: no such file or directory
--- FAIL: TestUsernsExecPS (0.15s)

And now in the test PR (opencontainers/runc#4112), all the tests pass. It is unfortunate that in theory we could produce the wrong data in this one case, but it's only for ancient kernels and (in runc's case at least) we are never running with heterogeneous threads when we are pid1 inside the container.

Copy link
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kolyshkin kolyshkin merged commit 4950d76 into moby:main Nov 7, 2023
9 checks passed
@cyphar cyphar deleted the mountinfo-linux-proc-self-fallback branch November 7, 2023 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants